[WEB-6813] fix: module not associated when accepting intake work items#8839
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
📝 WalkthroughWalkthroughExtracted cycle and module mutation logic from the issue modal into two helper functions, Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes intake acceptance updates so selected modules (and cycles) are properly associated with the accepted work item, and refactors the update flow by extracting cycle/module association logic into dedicated helpers.
Changes:
- Refactors cycle/module update side-effects into
handleCycleChangeandhandleModuleChange. - Fixes module association for intake acceptance by removing the previous
data.module_ids && payload.module_idsguard. - Runs cycle/module/property updates in parallel via
Promise.allafter the main issue update.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/core/components/issues/issue-modal/base.tsx`:
- Around line 323-336: The two follow-up mutations handleCycleChange and
handleModuleChange must run serially to avoid optimistic cache races; replace
the Promise.all call so you first await handleCycleChange(data, payload), then
await handleModuleChange(data, payload), and only after both complete call/await
handleCreateUpdatePropertyValues({ issueId: data.id, issueTypeId:
payload.type_id, projectId: payload.project_id, workspaceSlug:
workspaceSlug?.toString(), isDraft }); ensure you update the code around the
Promise.all to these sequential awaits so the optimistic writes from
handleCycleChange and handleModuleChange do not conflict.
- Around line 273-274: The two API calls issues.removeIssueFromCycle(...) and
fetchCycleDetails(...) (and the similar call around the other occurrence) pass
workspaceSlug as a possibly string|string[] from useParams(); coerce
workspaceSlug to a string before calling these helpers (e.g. use
workspaceSlug.toString() or String(workspaceSlug)) so the functions that expect
a string receive a definite string and satisfy TypeScript strict types; update
both call sites (removeIssueFromCycle and fetchCycleDetails and the similar call
around the later occurrence) to pass the coerced string value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53f0536d-d724-4906-9065-648e515dffbe
📒 Files selected for processing (1)
apps/web/core/components/issues/issue-modal/base.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/core/components/issues/issue-modal/base.tsx (1)
305-311: Consider renaming loop variable to avoid shadowingmoduleIdfromuseParams().The loop variable
moduleIdshadows the outermoduleIddestructured fromuseParams()at line 71. While this doesn't cause a bug (the outer variable isn't used inside this function), using a distinct name improves readability.♻️ Suggested rename
- for (const moduleId of updatedModuleIds) { - if (data.module_ids?.includes(moduleId)) { - modulesToRemove.push(moduleId); + for (const modId of updatedModuleIds) { + if (data.module_ids?.includes(modId)) { + modulesToRemove.push(modId); } else { - modulesToAdd.push(moduleId); + modulesToAdd.push(modId); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/issues/issue-modal/base.tsx` around lines 305 - 311, The for-loop in the updatedModuleIds iteration shadows the outer moduleId from useParams(); rename the inner loop variable in the loop over updatedModuleIds (currently "moduleId") to a distinct name such as "updatedModuleId" or "modId" and update its references inside the loop (the checks against data.module_ids and pushes to modulesToRemove/modulesToAdd) so that the outer moduleId remains unshadowed and readability is improved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/core/components/issues/issue-modal/base.tsx`:
- Around line 305-311: The for-loop in the updatedModuleIds iteration shadows
the outer moduleId from useParams(); rename the inner loop variable in the loop
over updatedModuleIds (currently "moduleId") to a distinct name such as
"updatedModuleId" or "modId" and update its references inside the loop (the
checks against data.module_ids and pushes to modulesToRemove/modulesToAdd) so
that the outer moduleId remains unshadowed and readability is improved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8575b095-9c5e-48ce-9e29-ac9d03c74e57
📒 Files selected for processing (1)
apps/web/core/components/issues/issue-modal/base.tsx
Re-apply 7 upstream fixes that needed careful merging with our Persian/RTL, i18n, and modules customizations. API - API issue update/create paths now dispatch model_activity for webhook delivery (fixes API webhooks not firing on PUT/PATCH/upsert) (makeplane#8792) Page navigation pane - Migrate tabs from headless ui to propel Tabs component (makeplane#8805) while preserving our RTL border-s/marginInlineEnd customizations - Add overflow scroll to outline (ScrollArea) and info tab panels (makeplane#8596), add px-4 to assets list, switch tab-panels root from Tab.Panels to Tabs.Content. fa locale already has sidebar.stickies/your_work — no i18n change needed Work item / editor - Issue modal: extract handleCycleChange and handleModuleChange helpers, fix module not associating when accepting intake items (WEB-6813, makeplane#8839) - Description input: add key prop to type def + force re-render via key={issue.id} in 3 call sites (WIKI-892, makeplane#8600) - Drag handle hover gap: containerClassName from "p-0 border-none" to "-ms-6 border-none p-0! ps-6!" — adapted to logical -ms/ps for RTL (WEB-6610, makeplane#8759) Profile - Profile cover update: align with correct Unsplash and upload handling, introduce "unsplash" image type, refactor analyzeCoverImageChange and handleCoverImageChange (WEB-6794, makeplane#8830). Preserved our t() i18n calls in form.tsx Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Description
had no effect — the module was never associated with the work item.
data.module_ids && payload.module_idsrequired both to be truthy. Intake items always have
nullmodule_ids(no modules yet), so the entire block was skipped.
Type of Change
Test Scenarios
Summary by CodeRabbit
Performance
Refactor